• 0b4f33c Use regex for case insensitive search, implement String instead of char<Vec> for seach_input, fix word disappearing by recalculating the render x for preview text.

Other than the last point, why would we want to do any of these things? Compiling a regex on each change to the search input makes the code less readable & negatively impacts performance compared to just taking the lowercase of the input, which we already do. Vec<char>, over String, is used to fix panics upon inputting a multi-byte unicode character, unicode_width is used to fix incorrect cursor positioning with wider unicode characters. Both are niche issues, but I don't see the purpose of removing these fixes for absolutely no improvement.

3b7e859 Replace redundant match statements with helper function in theme.rs.

Adding a new theme which has different colours for these theme elements could require these match statements be added back. This is the same thing you did when you suggested removing "unnecessary cases" in scripts. This change also does not improve readability.

Sorry, I misunderstood the changes made in that commit. It's even worse an idea than what I thought you were doing. If more themes are added, you'd have to keep adding function parameters, making the code even more difficult to read. A match statement is by far the best choice there.

I oppose these 2 commits. I'll review certain elements of the other changes in a few minutes.

tab completion was purposefully made to complete lowercase for UX & better visuals on the users end

i personally think its better to keep it lowercase as it feels more snappier that way imo, tldr: makes the user think they have more control when searching

• 0b4f33c Use regex for case insensitive search, implement String instead of char<Vec> for seach_input, fix word disappearing by recalculating the render x for preview text.

Other than the last point, why would we want to do any of these things? Compiling a regex on each change to the search input makes the code less readable & negatively impacts performance compared to just taking the lowercase of the input, which we already do. Vec<char>, over String, is used to fix panics upon inputting a multi-byte unicode character, unicode_width is used to fix incorrect cursor positioning with wider unicode characters. Both are niche issues, but I don't see the purpose of removing these fixes for absolutely no improvement.

Thanks for the quick review. I forgot about the multi-byte characters reverted back to Vec<char> with some comments.

@cartercanedy @lj3954 Why we are using ansi colors for tree-sitter-highlighting instead of ratatui colors?

treesitter-highlight provides the machinery for interacting with the individual parsers for syntax highlighting, in our case it's the bash parser. This is necessary

Sorry, misunderstood the initial question

Anstlye makes it easier to render the ansi color codes directly to a stream. There's some complication with using the ratatui text constructs since there might be some styling that needs to be applied over a line break, which would make parsing more complicated than necessary

Since we using custom colors for syntax_highlighting_styles maybe using the ratatui colors reduces the complexity?

What if ratatui colors is used and converted into lines using ratatui spans, lines or text instead of using anstyle and ansi_to_tui to convert them into text ?

Trust me, it's just going to get more complicated...

Ok I will try my best and create a commit let me know what you think about it

Is it normal for the checkout process to take in 10min in typos workflow 🙄 https://github.com/ChrisTitusTech/linutil/actions/runs/11782804516/job/32818643344?pr=933

The initial checkout can get throttled, I've seen that happen in my own projects

The initial checkout can get throttled, I've seen that happen in my own projects

Are the changes to the anstyle good ? Did you review the changes?

Making the theme functions const doesn't accomplish anything useful; they'll still be evaluated at runtime since the theme is not known at compile time.

We also should not be merging PRs like this. I find it unlikely Chris will accept anything with a 1500 line diff.

Making the theme functions const doesn't accomplish anything useful; they'll still be evaluated at runtime since the theme is not known at compile time.

It may not be now. But if we decide to change how themes work instead of passing the default theme at run time making it as default during compile time will make this more efficient.

We also should not be merging PRs like this. I find it unlikely Chris will accept anything with a 1500 line diff.

I'm aware of that. What can we do? Do I create 9 separate PRs for each commit?

Making the theme functions const doesn't accomplish anything useful; they'll still be evaluated at runtime since the theme is not known at compile time.

It may not be now. But if we decide to change how themes work instead of passing the default theme at run time making it as default during compile time will make this more efficient.

I don't mind the idea of marking methods const when compatible, but I don't see any way that the theme will ever become a compile-time constant as long as it remains configurable at runtime. The only exception would be if We could make this work if we constructed instances of each theme statically and just passed around references to the one of the compile-time generated themes

We also should not be merging PRs like this. I find it unlikely Chris will accept anything with a 1500 line diff.

I'm aware of that. What can we do? Do I create 9 separate PRs for each commit?

Yes

man that diff is HUGE
@jeevithakannan2, the conflict-creator

man that diff is HUGE
@jeevithakannan2, the conflict-creator

I know we have put a lot of time into this. Separating the PRs will do the same. But this makes the program more efficient in many areas. Can't complain.

Didn't expect you to actually merge this huge diff but ok.